-
Notifications
You must be signed in to change notification settings - Fork 2
f32 #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Hi Gareth, can you please to clarify how these changes affect our code in UMEP for QGIS. As far as we can see, you have changed e.g. solweig_run.py. Have you tested this in QGIS environment? I think maybe we have missed this from a previous PR. Why are you making changes in dailyshading.py as this is not used in SOLWEIG? Please clarify? |
|
Hi @biglimp - for this pull request I tried to change all instances of saving rasters to use The changes to The changes to What would be good is to find a way to test the Another nice change would be to separate old code into a separate directory and to then auto format all current code using |
|
Hi. Thanks for this info. I think you point towards the issue that we cannot control. There is no good way to test within a QGIS environment without doing it manually. On another note, @nilswallenberg and I are discussing to create a separate pypi for the solweig model consisting of only the model itself, without any other workflow tools such as SVF-calculator. Then we can all use that pypi module to run the actual model. Do you see any issues with this? |
|
@biglimp apologies for the slow response. At the moment
For this to work, the Personally I would recommend to keep SOLWEIG and other core functions for SVF and shadows together so that it isn't necessary to install and manage multiple packages. Though it would be easier to maintain if old or dead code is removed (or moved to a |
Sorry for late reply. I see your point but nevertheless, SVF and wall algorithm are separate tools that users make use of for many other applications a part from SOLWEIG-calculation. SOLWEIG is a separate model and therefore, it would be better to maintain this separately. |
|
Pulled the f32 branch and tried in the osgeo env. Got the following error: Here are the python packages installed (python 3.12.9): Am I missing something? |
|
Just made a simple test so that it was not my VSCode who did something strange. the code below: gives below with no errors: |
|
Thanks for the detailed report and for testing this. [EDIT - updated below in the later response.] The suspected issueOSGeo4W includes its own GDAL installation. When SolutionIdeally, use a separate virtual environment instead of the OSGeo4W Python: cd C:\Users\<path>\Documents\PythonScripts\umep-core
uv venv --python 3.12
.venv\Scripts\activate
uv pip install -e .
python demos/solweig_gbg_test.pyOr with standard venv: python -m venv .venv
.venv\Scripts\activate
pip install -e .Alternative: Condaconda-forge maintains compatible versions of the geospatial stack: conda create -n umep -c conda-forge python=3.12 rasterio geopandas pyproj shapely
conda activate umep
pip install -e .Diagnostic toolIn the more recent umep-doctorLet me know if you run into anything else. |
Hi @biglimp - Happy New Year. Sorry I've been quiet. Shall I go ahead and create a new separate repository to split out the Solweig logic? This would make it easier to maintain if we can do a fresh, clean start. I would ideally also like to simplify the function calls with a new logic, but would like to also keep it compatible with any other wrappers, e.g. QGIS that might rely on it. Please let me know if you have any thoughts. |
Hi @biglimp - I took another look at this assuming you'd like to run it directly in the OSGEO4W environment: The common.py file in the Could you please give this a go and let me know if it works for you? (The latest changes are in the |
Hi. Yes, that would be good. This will make it much easier to incorporate changes in the model, separate from e.g. Walls algorithms etc. @nilswallenberg , and me are also in contact with a group in the US who have made a GPU version of the code that we would like to include. Nils, could you add the link to the repo that has this code or is it non-public still? |
Thanks for this. I will try this as soon as I find the time. @nilswallenberg , if you have time to try this before me, please let me know the outcome. |
|
Hi, This is their repo: https://github.com/nvnsudharsan/SOLWEIG-GPU Cheers, |
|
Hi @songololo I tried running in the OSGEO4W Python environment with success. I only ran into problems with "from pvlib.iotools import read_epw", but running without it worked without issues. QGIS version: 3.44.3-Solothurn Cheers, |
Many thanks. If we fix that import we could finally merge this. |
#22
Makes use of f32 for reading, writing, internal arrays.